refactor(storage): make onResponse async and improve retry error handling - #24
Open
thiyaguk09 wants to merge 11 commits into
Open
refactor(storage): make onResponse async and improve retry error handling#24thiyaguk09 wants to merge 11 commits into
thiyaguk09 wants to merge 11 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the resumable upload logic to make onResponse asynchronous and introduces a helper function formatRetryError to standardize retry error formatting. The reviewer suggested refactoring formatRetryError to eliminate code duplication between the Error instance and generic object handling blocks.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
thiyaguk09
requested review from
gurusai-voleti,
mahendra-google and
shubhangi-google
June 24, 2026 13:11
gurusai-voleti
approved these changes
Jun 25, 2026
thiyaguk09
force-pushed
the
fix/resumable-upload-error-serialization
branch
2 times, most recently
from
July 10, 2026 12:30
fb994fb to
794370b
Compare
thiyaguk09
force-pushed
the
fix/resumable-upload-error-serialization
branch
5 times, most recently
from
July 27, 2026 06:48
a162ec6 to
a797680
Compare
thiyaguk09
force-pushed
the
fix/resumable-upload-error-serialization
branch
5 times, most recently
from
July 31, 2026 13:49
c587bcd to
434e34f
Compare
…kflow (googleapis#9033) Update `discovery.yaml` to fix failures on future workflows. - Update the PR title and commit message to use `chore(bigquery)`, so release-please component can identify the change correctly. - Add a cleanup step that removes temporary `pnpm-lock.yaml` files and reverts unintended formatting changes to handwritten source files. - Sets the `git_dif` key to `.` to the correct path. > NOTE: since handwritten packages do not track pnpm lockfiles running pnpm install creates one.
Ran the following commands on librarian version 0.31.1: ``` go install github.com/googleapis/librarian/cmd/librarian@0.31..1 librarian install nodejs librarian generate --all ```
## Description Upgrade all generated packages to Node 22 along with their dependencies. For all `package.json` files inside `packages/**` folder we set "node": ">=22" for `engines`. We also make sure that for all of these `package.json` files the dependencies that match the core libraries are all on the latest version so that all core library dependencies correspond to node v22. Note that a branch update is likely required to get the zizmor checks to pass. Also note that upgrading the auth library dependencies causes compiler errors so we opt out of upgrading the auth library. ## Impact Encourages customers to adopt new Node version which avoids issues due to lack of Node 18 support. ## Next Steps Upgrade the auth dependencies that we chose not to upgrade this time due to the compiler error. Tracked [here](https://b.corp.google.com/issues/541318013).
Now that we've migrated to librarian for new API libraries, this can go away.
…pis#9030) While trying to get the [handwritten split project](googleapis#8934) finished, I ran into a lot of resource leaks in the bigtable system tests. These ended up being extensive enough changes that I moved them to their own PR (this one).
…googleapis#9023) ## Summary This PR implements the core `Client`, `Pool`, and `Query` execution interfaces for `@google-cloud/spanner-driver`, providing compatibility with `node-postgres` (`pg`) driver layer for Google Cloud Spanner --- ## Key Changes ### 1. `Client` Connection & Execution Queue (`src/lib/client.ts`) - **`Client` Class**: Implements `node-postgres` compatible `Client` handle managing connection state (`isConnected`), transaction status tracking (`txStatus: 'I' | 'T' | 'E'`), and DSN resolution. - **Sequential Task Queue (`queryQueue`)**: Enforces sequential query execution order per client connection handle. - **Transaction Status Lifecycle**: Updates `txStatus = 'I'` only after statement execution completes (`COMMIT`, `ROLLBACK`, `ABORT`). - **`release()` Method**: Added `client.release()` method delegating to connection teardown for `node-postgres` compatibility. - **Unhandled Error Prevention**: Added `query.listenerCount('error') > 0` check prior to emitting `'error'` events, preventing Node process crashes when queries are consumed via Promises (`await client.query()`). ### 2. `Query` Class & Thenable / EventEmitter Integration (`src/lib/query.ts`) - **Dual Invocation Model**: Extends `EventEmitter` for row streaming (`.on('row', cb)`, `.on('end', cb)`) while implementing the Thenable interface (`then`, `catch`, `finally`) for `async`/`await` support. - **Overload Support**: Supports query strings, `QueryConfig` objects, `Query` instances, positional value arrays (`$1`, `$2`), and Node callbacks (`(err, res) => void`). - **Constructor Robustness**: - Added `text !== null` guard when initializing from objects (`typeof text === 'object' && text !== null`). - Added support for overriding positional `values` and `callback` when instantiating from an existing `Query` instance. ### 3. `Pool` Scaffolding & Callback Single-Invocation Rule (`src/lib/pool.ts`) - **`Pool` Class**: Implements `connect()`, `query()`, and `end()`. - **Client Binding**: Binds `client.release = client.end.bind(client)` during client acquisition with `TODO(PR 4 - Connection Pooling)` markers for pool recycling in PR 4. - **Single Invocation Guarantee**: Isolated `_doConnect()` connection acquisition error handling from query execution, ensuring connection failures call callbacks exactly once without hanging or double callbacks. - **3rd-Argument Callback Overload Resolution**: Resolved 3rd-argument callback parameters when executing `pool.query(query, values, callback)` or `client.query(query, values, callback)`.
… for retry failures
- Unify formatting of Errors and objects under a single block inside formatRetryError to reduce code duplication. - Ensure standard errors, GaxiosErrors, and custom errors with empty/missing properties are correctly formatted.
thiyaguk09
force-pushed
the
fix/resumable-upload-error-serialization
branch
from
August 3, 2026 07:12
434e34f to
5a7ab72
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕